Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64: Add {u,s}mulhi.i8 instruction support #7866

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

afonso360
Copy link
Contributor

👋 Hey,

This PR adds support for smulhi.i8 and umulhi.i8 to the x64 backend.

Closes #5468
Closes #7865

@afonso360 afonso360 added the cranelift:area:x64 Issues related to x64 codegen label Feb 3, 2024
@afonso360 afonso360 requested a review from a team as a code owner February 3, 2024 19:10
@afonso360 afonso360 requested review from fitzgen and removed request for a team February 3, 2024 19:10
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Feb 3, 2024
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@alexcrichton alexcrichton added this pull request to the merge queue Feb 3, 2024
Merged via the queue into bytecodealliance:main with commit c7a7b8c Feb 3, 2024
26 checks passed
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Feb 3, 2024
This commit is inspired after reading over some code from bytecodealliance#7865
and bytecodealliance#7866. The goal of this commit was to refactor
scalar multiplication-related instructions in the x64 backend to more
closely align with their native instructions. Changes include:

* The `MulHi` instruction is renamed to `Mul`. This represents either
  `mul` or `imul` producing a doublewide result.
* A `Mul8` instruction was added to correspond to `Mul` for the 8-bit
  variants that produce a doublewide result in the `AX` register rather
  than the other instructions which split between `RAX` and `RDX`.
* The `UMulLo` instruction was removed as now it's covered by `Mul`
* The `AluRmiROpcode::Mul` opcode was removed in favor of new `IMul` and
  `IMulImm` instructions. Register allocation and emission already had
  special cases for `Mul` which felt better as standalone instructions
  rather than putting in an existing variant.

Lowerings using `imul` are not affected in general but the `IMulImm`
instruction has different register allocation behavior than before which
allows the destination to have a different register than the first
operand. The `umulhi` and `smulhi` instructions are also reimplemented
with their 8-bit variants instead of extension-plus-16-bit variants.
github-merge-queue bot pushed a commit that referenced this pull request Feb 9, 2024
* x64: Refactor multiplication instructions

This commit is inspired after reading over some code from #7865
and #7866. The goal of this commit was to refactor
scalar multiplication-related instructions in the x64 backend to more
closely align with their native instructions. Changes include:

* The `MulHi` instruction is renamed to `Mul`. This represents either
  `mul` or `imul` producing a doublewide result.
* A `Mul8` instruction was added to correspond to `Mul` for the 8-bit
  variants that produce a doublewide result in the `AX` register rather
  than the other instructions which split between `RAX` and `RDX`.
* The `UMulLo` instruction was removed as now it's covered by `Mul`
* The `AluRmiROpcode::Mul` opcode was removed in favor of new `IMul` and
  `IMulImm` instructions. Register allocation and emission already had
  special cases for `Mul` which felt better as standalone instructions
  rather than putting in an existing variant.

Lowerings using `imul` are not affected in general but the `IMulImm`
instruction has different register allocation behavior than before which
allows the destination to have a different register than the first
operand. The `umulhi` and `smulhi` instructions are also reimplemented
with their 8-bit variants instead of extension-plus-16-bit variants.

* Remove outdated emit tests

These are all covered by the filetests framework now too.

* Fix Winch build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
2 participants